Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/perf_ci_linstress_vm_ODF_PVC_False/linstress_vm.yaml`:
- Around line 9-14: The VM template still hardcodes a known login password in
the linstress cloud-init data, so update the relevant template generation path
in linstress_vm_template.yaml to remove the fixed password and disable password
auth by default. Use key-based authentication or inject a per-run secret through
the template parameters instead, and ensure the generated golden file reflects
the updated behavior.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@benchmark_runner/common/template_operations/templates/linstress/internal_data/linstress_vm_template.yaml`:
- Around line 78-96: The monitoring loop in the linstress template is
oversleeping for short runs because `intervals = max(1, duration // 30)` always
forces at least one 30-second wait, so `burn_cpu` and `burn_memory` can finish
before sampling starts. Update the loop in the monitoring section to use
elapsed-time tracking against the requested `duration` instead of a fixed
interval count, and only sleep for the remaining time needed before each sample.
Keep the sampling logic and `cpu_procs`/`mem_proc` joins in place, but make sure
the first sample is taken within the actual stress window even when `duration <
30`.

In
`@benchmark_runner/common/template_operations/templates/winstress/internal_data/01_run_stress_template.ps1`:
- Line 6: The $reportFile variable in 01_run_stress_template.ps1 is assigned but
never reused, while the report path is hardcoded again later in the script.
Update the logic around $reportFile so the same variable is used wherever the
stress report path is needed, including the report-writing and report-reading
spots in the script, and remove the duplicated literal path to keep it
consistent with $stressDir.

In
`@benchmark_runner/common/template_operations/templates/winstress/internal_data/windows_dv_template.yaml`:
- Line 6: The DataVolume name in the Windows clone template is hardcoded and
shared across multiple Windows workloads, which can cause collisions during
concurrent runs. Update the name in windows_dv_template.yaml to be unique per
workload or per run by incorporating a suffix such as the existing template
uuid/identifier, and make sure the Windows clone path stays consistent in the
template that defines the DataVolume object.

In `@benchmark_runner/workloads/linstress_vm.py`:
- Around line 35-43: _remove the per-VM error-log call from the multiprocessing
phase helpers and let the top-level run() failure handling own it. In
_create_vm, _collect_results, and _delete_vm, avoid calling
self.save_error_logs() inside the child-process path; instead, just propagate
the exception and keep failure logging centralized in run()’s except block,
which already runs in the main process. Use the existing _run_parallel_phases
flow and the BootstormVM-style pattern as the reference for locating these step
functions._
- Around line 64-70: The SCP/report path in linstress_vm’s result handling is
swallowing failures by only logging a warning and returning from _scp_file
handling, which lets _upload_results skip failed VMs and still complete
successfully. Update the failure path around the stress report fetch in
linstress_vm to track missing-result/SCP failures per VM, and have the workload
lifecycle logic raise an exception or otherwise mark the run failed when no
usable results are produced. Make sure the fix is wired through the existing
_upload_results flow so repeated VM failures are not silently ignored.
- Around line 54-62: The polling loop in linstress_vm is using a range counter
with OC.DELAY, but the virtctl_ssh call can take much longer than that, so the
real wait can exceed self._timeout. Update the timeout logic in the stress
report check loop to use a wall-clock deadline or elapsed-time tracking around
virtctl_ssh and time.sleep, so the loop exits once the overall timeout is
reached. Keep the change localized to the polling block that calls
self._virtctl.virtctl_ssh and logs the “Waiting for stress_report.json” message.

In `@benchmark_runner/workloads/winstress_vm.py`:
- Around line 65-91: The failure handling in `_prepare_vm` and `_run_stress` is
inconsistent with `_create_vm`, `_collect_results`, and `_delete_vm`, because
these methods only log warnings and swallow exceptions. Update `_prepare_vm` and
`_run_stress` in `winstress_vm.py` to propagate failures by re-raising after
logging, so a VM that never becomes SSH-ready or fails during stress does not
continue into later phases and produce misleading results.
- Around line 98-106: The wait loop in winstress_vm’s stress_report.json polling
uses a fixed iteration count instead of real elapsed time, so the combined
virtctl_ssh timeout and sleep can exceed the intended self._timeout budget.
Update the polling in the method that builds check_cmd and calls
virtctl.virtctl_ssh to track wall-clock elapsed time (or a deadline) and stop
once the total time reaches self._timeout, keeping the per-check SSH timeout
bounded as well. Ensure the logger message and break condition still reflect the
same stress_report.json check logic.
- Around line 57-63: The worker helper error handlers in _create_vm(),
_collect_results(), and _delete_vm() are doing failure logging via
save_error_logs(), which should not run inside multiprocessing.Process workers.
Remove the save_error_logs() call from those except blocks and keep them only
re-raising the exception, then update run() to perform the error logging after
_run_parallel_phases() returns so the Elasticsearch upload happens in the parent
process.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/func_ci_linstress_vm_ODF_PVC_False/linstress_vm.yaml`:
- Around line 8-12: The cloud-init settings in the linstress VM template use a
hardcoded `user`/`password` pair with `ssh_pwauth` enabled, which should be
removed from this benchmark fixture. Update the `linstress_vm.yaml` template to
stop using fixed credentials in this cloud-config block and instead follow the
same safer approach used by the Windows workload (prefer SSH public key
injection and disable password auth), or at minimum generate a per-run random
password rather than a literal default.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/func_ci_linstress_vm_ODF_PVC_True/linstress_vm.yaml`:
- Around line 8-12: The cloud-init rendered by the linstress VM template is
using hardcoded weak credentials and enabling SSH password login. Update the
linstress cloud-init/template that produces linstress_vm.yaml so it no longer
sets a fixed user/password pair and does not enable ssh_pwauth; switch the
template to key-based auth only or generate a unique per-run password. Use the
cloud-init generation path associated with linstress_vm and the template/data
source that fills user, password, and ssh_pwauth to locate the fix.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/perf_ci_linstress_vm_ODF_PVC_False/linstress_vm.yaml`:
- Around line 6-12: The VM userdata in the linstress template hardcodes the
fedora account password, keeps it non-expiring, and enables password SSH auth.
Update the cloud-config in the linstress VM template so it relies on the SSH key
flow used by linstress_vm.py, and remove or template the password-related fields
and ssh_pwauth setting instead of shipping fixed credentials.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/release_linstress_vm_ODF_PVC_False/linstress_vm.yaml`:
- Around line 9-12: The cloud-init template for the linstress VM is using
hardcoded weak SSH credentials and password auth, which should be removed for
benchmark_runner artifacts. Update the template behind the linstress_vm.yaml
asset so the user/password values are not static (for example, inject a per-run
secret through the data template or switch the VM to SSH key auth), and keep the
relevant cloud-init fields such as ssh_pwauth and chpasswd aligned with that
change.

---

Nitpick comments:
In `@benchmark_runner/common/template_operations/template_operations.py`:
- Around line 156-173: The new workload_config handling duplicates the existing
hammerdb_config override flow in TemplateOperations, so extract the shared
“filter empty values, detect unknown keys, log, and merge into render_data”
behavior into a private helper. Reuse that helper from the hammerdb_config and
workload_config branches in template_operations.py so both paths stay consistent
and future changes only need to be made once. Keep the workload_name condition
for hammerdb_config in place, and preserve the existing
logger.warning/logger.info behavior through the shared helper.

In
`@benchmark_runner/common/template_operations/templates/winstress/internal_data/winstress_vm_template.yaml`:
- Around line 4-12: The VM name/label logic in the template is duplicated in
multiple places and should be centralized for consistency. In
winstress_vm_template.yaml, add Jinja {% set %} variables near the top for the
computed VM name and root disk name, then replace the repeated {% if scale
%}...{% else %}...{% endif %} blocks in the template sections with those shared
variables (for labels, disks, volumes, and dataVolumeTemplates). Use the
existing template symbols like kind, trunc_uuid, and scale to build vm_name and
root_disk_name once, then reference them everywhere else.

In `@benchmark_runner/main/environment_variables.py`:
- Around line 107-128: The workload list in the help text/README is missing the
newly supported `winstress_vm` and `linstress_vm` entries added in
`EnvironmentVariables`; update the documentation that describes the WORKLOAD
parameter so it matches the values in `environment_variables_dict['workloads']`.
Locate the workload documentation section referenced by the coding guidelines
and add these two names alongside the existing workload variants, keeping the
list consistent with the implementation.

In `@benchmark_runner/workloads/linstress_vm.py`:
- Line 53: The check_cmd assignment in linstress_vm.py is using an unnecessary
f-string prefix even though there are no placeholders. Update the check_cmd
string literal in the workload setup code to a plain string so it no longer
triggers Ruff F541, keeping the same shell command content.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/chaos_ci_winstress_vm_ODF_PVC_False/winstress_vm.yaml`:
- Around line 96-102: The `winstress_vm.yaml` cloud-init `runcmd` block briefly
opens `.ssh` and `authorized_keys` to `Everyone:F`, which should be removed.
Update the `userData` setup steps so the existing file can be taken over and
deleted using a SYSTEM/Administrator-only approach in the same `runcmd`
sequence, then keep the final restrictive ACL restoration in place. Reference
the `userData` SSH setup commands to locate the temporary permission grant and
replace it with a non-permissive ownership reset flow.

In
`@tests/unittest/benchmark_runner/common/template_operations/golden_files/func_ci_linstress_vm_ODF_PVC_True/linstress_vm.yaml`:
- Around line 126-129: The runcmd in the linstress_vm.yaml template is
redundantly reinstalling python3-psutil even though it is already declared in
the packages section. Update the runcmd block in the linstress_vm template to
remove the duplicate dnf install step, or keep it only if truly needed as a
fallback and add a brief inline comment explaining why. Use the runcmd and
packages entries in func_ci_linstress_vm_ODF_PVC_True/linstress_vm.yaml to
locate the change.